Improvement/global polling - #61
Merged
Merged
Conversation
generate_docker()/generate_workflow_docker() now recursively sweep every .py file under the project directory into the build context, preserving directory structure, so helper files that aren't declared as an agent entrypoint still make it into the image. Generated dirs (docker_container/, stubs/, grpc_stubs/) are excluded at the project root only, not at every depth. Stub files are placed at their agent's declared entrypoint path (mapped from global_controller.yaml) instead of a hardcoded guess, so a stub overwrites the exact real file it replaces. Guards against absolute and '..'-containing entrypoints, symlinked sources, and symlinked-destination escapes, with warnings on unsafe or unmapped stubs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…heckpoint) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> # Conflicts: # examples/portfolio/agents/advisor_agent.py
Users had no way to get API keys (OpenAI, Anthropic, embedding models) into an agent container. Add a top-level `env_file` key to global_controller.yaml pointing at a local .env file, which reaches every container as `docker run --env-file`. - resolve_env_file validates the path before anything launches, so a missing .env fails at deploy time instead of deep inside a container. Relative paths resolve against the project root, matching entrypoint. - env_file_args is a context manager owning the local-vs-remote decision and the cleanup, so both runtimes share one code path. Local containers read the original file; remote containers get a copy that is deleted as soon as `docker run` returns, whether or not it succeeded. - GlobalController._push_file streams the file over ssh under `umask 077` rather than scp, so the copy is never briefly world-readable and the secret never lands in a command line. _run_cmd's ssh options moved to a shared _ssh_args. --env-file is appended after the explicit -e VENTIS_* flags; Docker gives those precedence regardless of order, so a stray VENTIS_* line in someone's .env cannot break agent wiring. Closes #50
Two holes in the remote staging path, both found reviewing the feature commit. `umask 077` only governs files the shell creates, and `>` follows symlinks -- so it did not actually guarantee a 0600 copy. The destination path is fully predictable (`/tmp/ventis-env-ventis-ec2-<agent>-<n>`), so a local user on the remote host could pre-create it world-readable, or point it at a file of their own, and collect the API keys. Remove whatever sits at the path before writing; `rm -f` unlinks a symlink rather than following it, so `cat >` then creates a fresh file under the umask. `_run_cmd` joins its argv with spaces and hands the result to a remote shell unquoted. `_push_file` quoted its path but the cleanup `rm` did not, so a container name containing a space split the `rm` into two arguments that matched nothing -- it exited 0 while the secrets file stayed on the host, and the returncode check logged nothing. Scrub the name down to [A-Za-z0-9_.-] in remote_env_path, which also closes the same gap in the `--env-file` argument and in any future use of that path. Still open, tracked separately: a push that dies mid-transfer can leave a copy behind, since the cleanup only covers the `docker run` that follows. On EC2 the instance is terminated on that path, which disposes of it.
…nup-race fix (pre-pull checkpoint) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This project's agents/workflow import each other's stubs by flat module name, not by the exporting agent's own entrypoint path. Applying _stub_destination's entrypoint-mirroring broke both the Workflow (ModuleNotFoundError: intent_agent) and agent-to-agent calls (MetricsAgent -> price_agent) on live redeploy. Keeps PR #51's actual fix (project_dir sweep for unstubbed helper files) intact. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Metrics/telemetry latency scaled with instance count x per-instance round-trip time since every instance was polled sequentially, one blocking the next, with the following tick only starting after the whole pass finished. Extracted the per-instance body into _poll_one_instance (whole body wrapped in one top-level try/except, since ThreadPoolExecutor.map() re-raises on first exception when results are consumed) and run all instances concurrently via the same ThreadPoolExecutor pattern _trigger_cleanup already used. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Metrics/telemetry latency scaled with instance count x per-instance round-trip time since every instance was polled sequentially, one blocking the next, with the following tick only starting after the whole pass finished. Extracted the per-instance body into _poll_one_instance (whole body wrapped in one top-level try/except, since ThreadPoolExecutor.map() re-raises on first exception when results are consumed) and run all instances concurrently via the same ThreadPoolExecutor pattern _trigger_cleanup already used. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Resolved conflicts: - metrics_agent.py: removed duplicate imports - portfolio_workflow.py: used main's simpler intent handling - cli.py: kept both comment explanations - stub_generator.py: used main's version (no entrypoint injection)
Brings in all OTel cleanup work: - Removed legacy single-destination fallback - Removed langfuse Basic-Auth auto-injection - Updated DESIGN.md to remove legacy references - Simplified config env-var expansion - Fixed langfuse config examples - Merged latest from main (joke_writer, porting skill, etc.) All conflicts resolved by taking feature/otel-exporter's cleaner versions.
… and remove _write_entrypoint_file - Fixed langfuse example to use generic env-var headers pattern - Removed _write_entrypoint_file (directory structure preservation via _sweep_py_files is cleaner)
Re-applied the parallel instance polling that was lost during conflict resolution. The _poll_controllers method now uses ThreadPoolExecutor to poll all instances concurrently via _poll_one_instance, preventing one slow instance's Redis/Postgres round-trip from blocking the entire poll tick.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Resolves two conflicts: - global_controller.py: _poll_controllers() conflicted with main's sequential loop because this branch parallelized it (ThreadPoolExecutor + a new _poll_one_instance() method). Diffed both versions ignoring whitespace and confirmed every line of actual per-instance logic (OTEL write_waiting_rows, send_runtime_information, metrics, status) is byte-identical between branches -- this branch's version is a strict superset (same logic, parallelized), so kept it wholesale and left every other line in the file matching main exactly. - env_file.py (add/add): same conflict as PR #60, same fix -- took main's version, a superset that adds platform_secrets_file() on top of the unchanged resolve_env_file(config, base_dir) API this branch already calls. Also fixes the same pre-existing SyntaxError found on feature/otel-exporter (shared ancestor): cmd_build()'s generate_workflow_docker() and generate_docker() calls each repeated project_dir= and stub_entrypoints= as duplicate keyword arguments. ventis/cli.py could not be imported/ compiled before this fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Saaketh0
added a commit
that referenced
this pull request
Sep 9, 2026
fixes/ventis-cli-fixes and feature/config-reloading are parallel lines off a common ancestor far back (75d7705), not a linear descendant relationship -- each independently re-implemented overlapping OTel/otel-exporter work after diverging. Resolved per-file based on which side is the actual superset, verified with full-file diffs against both parents, not a blanket "one side wins" rule: - env_file.py (add/add): took feature/config-reloading's side -- it already has the managed-secrets platform_secrets_file()/DEFAULT_SECRETS_FILE support merged in from main (via this session's PR #60/#61 work), which fixes/ventis-cli-fixes predates and lacks entirely. - db.py, convert.py, test_otel_exporter_fields.py (add/add): took fixes/ventis-cli-fixes -- cost-lookup try/except fallback and 64-bit Future.id/span_id, both strict additions config-reloading never touched. - _runtime.py: took fixes/ventis-cli-fixes -- adds a port-conflict retry loop around docker run; verified the whole-file diff is confined to this one function, and that registering the Redis endpoint AFTER the retry loop (instead of before, like config-reloading) is required once host_port can change mid-retry, not just a style choice. - global_controller.py: took fixes/ventis-cli-fixes -- verified via full diff (not just the 5 conflict markers) that every other difference in the file, conflicted or already auto-merged, also favors this side (.car-layout project root resolution, persisted dashed-uuid project_id, VENTIS_REDIS_HOST env override for a containerized GC's own Redis connection). - cli.py, test_cli.py: took fixes/ventis-cli-fixes -- folds build into deploy per this PR's own description, fixes entrypoint-only stub placement, and incidentally fixes the recurring duplicate-keyword SyntaxError (project_dir=/stub_entrypoints= repeated) that config-reloading's copy still carries since it branched before that got fixed elsewhere this session. - otel_exporter.py (add/add): user decision -- kept fixes/ventis-cli-fixes's RedisClient(host="host.docker.internal") as-is, despite it not matching the VENTIS_REDIS_HOST env-var pattern this same PR introduces for GlobalController's own Redis connection a few lines away in global_controller.py. Flagged as a known inconsistency, not fixed here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added concurrency to the polling function in global controller. Added ThreadPoolExecutor.
Large diff is mainly just wrapping things into new blocks and indents, not much actual changes.